Skip to content

Conversation

@stephentoub
Copy link
Member

It's reasonably common for a codebase to pick a specific newline string and set it regardless of OS, but currently that ends up allocating on every OS each call to set_NewLine.

Two changes:

  1. When setting TextWriter.NewLine to the existing value, make it a nop to avoid unnecessarily calling ToCharArray.
  2. When setting TextWriter.NewLine to "\n" on Windows or to "\r\n" on Unix, use a cached array.

Copilot AI review requested due to automatic review settings November 10, 2025 21:19
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 10, 2025
@stephentoub stephentoub added area-System.IO and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Nov 10, 2025
Copilot finished reviewing on behalf of stephentoub November 10, 2025 21:21
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

It's reasonably common for a codebase to pick a specific newline string and set it regardless of OS, but currently that ends up allocating on every OS.

Two changes:
1. When setting TextWriter.NewLine to the existing value, make it a nop to avoid unnecessarily calling ToCharArray.
2. When setting TextWriter.NewLine to "\n" on Windows or to "\r\n" on Unix, use a cached array.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR optimizes TextWriter.NewLine setter to reduce allocations in common scenarios where codebases set a specific newline string regardless of OS. The changes include:

  1. Making the setter a no-op when setting to the same value
  2. Caching arrays for "\n" and "\r\n" (the non-default OS newline strings)

@stephentoub stephentoub merged commit 984128f into dotnet:main Nov 17, 2025
134 of 142 checks passed
@stephentoub stephentoub deleted the newline branch November 17, 2025 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants